Add old, new, diff and operation to WebhookCause #857
Conversation
sambhav
force-pushed
the
webhook-cause
branch
from
d62e9a6 to
36fa21d
Compare
kopf/_core/engines/admission.py
Outdated
| old = bodies.BodyEssence(old_body) if old_body is not None else None | ||
| new = bodies.BodyEssence(new_body) if new_body is not None else None |
There was a problem hiding this comment.
Thanks for the contribution. Generally, looks great and the feature might be useful.
The only concern is here, in these lines. The body essence is different from the body — the essence removes all the garbage. The definition of "garbage" is flexible, but mostly it is the system metadata: e.g. generation/resourceVersion/timestamps. I'm not sure how Kubernetes passes the metadata in admission hooks — I simply didn't pay attention to it before. So, I will perform some experiments locally to see how it fits and feels before merging.
As a side-note (no action is needed; for my own remembering later): In the change-detecting handlers, the essence also removes the status stanza and re-adds the explicitly monitored fields. But that was done for the purpose of not (over-)reacting to non-essential changes, i.e. to dummy/technical events or other controllers storing their status fields (or even annotations of other Kopf-based controllers). In the case of admission, the reaction happens anyway, so there is no big win in removing the status/annotations. It can actually be more confusing if a change is admitted, but the diff is empty — so, it should not be made empty. [/the end of the side-note]
There was a problem hiding this comment.
Would you prefer this to be the body or the body essence? I am good with either.
|
PS: The massive unit-test failure is probably caused by 3rd-party changes in |
|
Please rebase on |
sambhav
force-pushed
the
webhook-cause
branch
from
36fa21d to
68f5256
Compare
|
@nolar done :) |
sambhav
force-pushed
the
webhook-cause
branch
2 times, most recently
from
c8c24fd to
9abdb2c
Compare
Adding kwargs old, new, diff and operation to the WebhookCause and passing them to the appropriate handler. This allows users to write more powerful webhooks and allows them to create validation and mutation webhooks that can be look at the old object to determine their behavior. This commit also adds the operation field to the passed kwargs to allow users to create more powerful/flexibly wenhooks. Signed-off-by: Sambhav Kothari <sambhavs.email@gmail.com>
sambhav
force-pushed
the
webhook-cause
branch
from
9abdb2c to
027880c
Compare
|
@nolar updated it to use bodies.Body instead. That should also fix the CI mypy issue. |
|
@nolar just a ping in case this PR slipped through your inbox :) |
|
@samj1912 Sorry for the delay. Don't worry, I didn't forget it — just working hard on another issue (which finally works!). |
|
Released as 1.35.3 |
|
Amazing! Thank you so much! |
Adding kwargs old, new, diff and operation to the WebhookCause and
passing them to the appropriate handler. This allows users to write
more powerful webhooks and allows them to create validation and
mutation webhooks that can look at the old object to determine
their behavior. This commit also adds the operation field to the
passed kwargs to allow users to create more powerful/flexibly webhooks.
closes: #851